Skip to content

Fix SCMOSCamera matrix convention to match Julia standard#34

Merged
kalidke merged 2 commits intomainfrom
fix/scmos-matrix-convention
Jan 9, 2026
Merged

Fix SCMOSCamera matrix convention to match Julia standard#34
kalidke merged 2 commits intomainfrom
fix/scmos-matrix-convention

Conversation

@kalidke
Copy link
Member

@kalidke kalidke commented Jan 9, 2026

Summary

  • Fixed _validate_camera_param to expect (ny, nx) instead of (nx, ny) for calibration matrices
  • The previous convention was inconsistent with Julia's [row, col] indexing
  • This caused BoundsError for rectangular cameras and semantic errors (transposed data) for square cameras

Details

The validation check for calibration matrices (readnoise, gain, offset, qe) was expecting size(param) == (nx, ny), but Julia arrays use A[row, col] indexing. For a camera with nx columns and ny rows, matrices should be (ny, nx).

Bug impact:

  • For rectangular cameras: BoundsError when downstream code (like GaussMLE) accesses map[y, x]
  • For square cameras: No error, but data semantically transposed - pixel (x,y) gets value meant for (y,x)

Why it wasn't caught:

  • Most tests used square cameras where (nx, ny) == (ny, nx)
  • SMLMData is a container that doesn't access matrix elements itself

Test plan

  • Existing tests pass (fixed the one rectangular test that used wrong convention)
  • Added explicit test for rectangular camera verifying semantic access map[y, x]
  • Added test that transposed matrix throws DimensionMismatch

🤖 Generated with Claude Code

kalidke and others added 2 commits January 9, 2026 09:40
The validation check for calibration matrices (readnoise, gain, offset, qe)
was expecting size (nx, ny) but Julia arrays use [row, col] indexing, so
matrices should be (ny, nx) where ny=rows and nx=cols.

This bug was masked by square camera tests but would cause:
- BoundsError for rectangular cameras when downstream code accesses map[y, x]
- Semantic errors for square cameras (data transposed)

Changes:
- Fix _validate_camera_param to expect (ny, nx) instead of (nx, ny)
- Update docstrings to clarify the convention
- Fix existing rectangular test that used wrong convention
- Add explicit test verifying semantic access pattern

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@kalidke kalidke merged commit 919326b into main Jan 9, 2026
6 checks passed
@kalidke kalidke deleted the fix/scmos-matrix-convention branch January 9, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant